-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor unlinked files #7209
Refactor unlinked files #7209
Conversation
create dialog in fxml and use a background task for import
…dtask * upstream/master: Bump pdfbox from 2.0.21 to 2.0.22 (#7213) Bump fontbox from 2.0.21 to 2.0.22 (#7219) Bump archunit-junit5-api from 0.14.1 to 0.15.0 (#7220) Bump unoloader from 7.0.3 to 7.0.4 (#7214) Bump guava from 30.0-jre to 30.1-jre (#7218) Bump xmpbox from 2.0.21 to 2.0.22 (#7217) Bump classgraph from 4.8.94 to 4.8.97 (#7211) Bump byte-buddy-parent from 1.10.18 to 1.10.19 (#7216) Bump archunit-junit5-engine from 0.14.1 to 0.15.0 (#7215) Bump org.beryx.jlink from 2.22.3 to 2.23.0 (#7212) Add missing author Remove field check for journal abbrev in entry editor (#7208)
copy over some methods
TODO: Import Button does not yet work, always null
TODO: Progress not yet shown
adjust dialog
…dtask * upstream/master: add language mapping for chinese remove chinese content fix hamcrest link Add Traditional Chinese (#7240) Show development information Allow manual trigger of the deployment workflow Release v5.2 Adapt changelog for 5.2 release Update external-libraries.md checkstyle L10n master (#7235) fix fetcher architecture test Add missing author Add error dialog "Problem finding files" (#6842) Disable ACM, Google Scholar, JSTOR (#7229)
disable import button
…dtask * upstream/master: GitBook: [master] 5 pages and 25 assets modified New Crowdin updates (#7246)
Any more things left or can we merge? Think it looks good now |
I am biased, so i can't judge... 😉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some nitpicks 😇
src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialog.fxml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few remarks!
src/main/java/org/jabref/gui/externalfiles/ImportFilesResultItemViewModel.java
Outdated
Show resolved
Hide resolved
private final FileUpdateMonitor fileUpdateMonitor; | ||
private final ExternalFilesEntryLinker linker; | ||
private final ExternalFilesContentImporter contentImporter; | ||
private final UndoManager undoManager; | ||
private final StateManager stateManager; | ||
private List<ImportFilesResultItemViewModel> results; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still think this, why did you marked it as "resolved"?
scanDirectoryValidator = new FunctionBasedValidator<>(directoryPath, isDirectory, | ||
ValidationMessage.error(Localization.lang("Please enter a valid file path."))); | ||
|
||
treeRootProperty.setValue(null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use the EasyBind.OptionalProperty instead of handling with null values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I think it's necessary to prevent showing any empty things
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The optional property is similar to Optional
. Just a nice wrapper so that you don't need null
but have an isEmpty
etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took a closer look into the EasyBind
package, although there is an OptionalBinding
, there seems to be no OptionalProperty
. I took the liberty to make this an ObjectProperty<Optional<FileNodeViewModel>>
.
src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModel.java
Outdated
Show resolved
Hide resolved
…dtask * upstream/master: Upgrade citeproc to 3.x snapshot without graal (#7370) Fix Exception if no AzureInstrumentationKey is available (#7373) Update snapcraft source url (#7372) Fix checkstyle and adjust language GitBook: [master] 3 pages and 32 assets modified Add migration to special field (#7368) GitBook: [master] 5 pages and 29 assets modified Modify message at the duplicates found dialog (#7231) Fixes miss-parsed names in `AutomaticPersonsGroup` (#7228) Fix an issue where the password for a shared SQL database was only remembered if it was the same as the username (#7364) Fix harvard exporter by changing AuthorsFormatter (#7355) Bump styfle/cancel-workflow-action from 0.6.0 to 0.7.0 (#7363) Bump mockito-core from 3.7.0 to 3.7.7 (#7360) Bump org.beryx.jlink from 2.23.1 to 2.23.2 (#7361) Bump libreoffice from 7.0.3 to 7.0.4 (#7362) Export full month name instead of number in ms office (#7358)
…mporterbackgroundtask * upstream/importerbackgroundtask: Remove obsolete language key Refactored for mvvm pattern and optics # Conflicts: # src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogView.java # src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModel.java
and l10n
add explaination to checkstyle
Addressed all comments except for those Binding or Optional stuff, cause I can't get it to work... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK thanks!
* upstream/master: (217 commits) Fix handling of URL in file field (JabRef#7347) Fix expansion of bracketed expressions in RegExpBasedFileFinder (JabRef#7338) Refactor unlinked files (JabRef#7209) Add pressing enter when the search field is focused as a way to trigger search (JabRef#7377) Upgrade citeproc to 3.x snapshot without graal (JabRef#7370) Fix Exception if no AzureInstrumentationKey is available (JabRef#7373) Update snapcraft source url (JabRef#7372) Fix checkstyle and adjust language GitBook: [master] 3 pages and 32 assets modified Add migration to special field (JabRef#7368) GitBook: [master] 5 pages and 29 assets modified Modify message at the duplicates found dialog (JabRef#7231) Fixes miss-parsed names in `AutomaticPersonsGroup` (JabRef#7228) Fix an issue where the password for a shared SQL database was only remembered if it was the same as the username (JabRef#7364) Fix harvard exporter by changing AuthorsFormatter (JabRef#7355) Bump styfle/cancel-workflow-action from 0.6.0 to 0.7.0 (JabRef#7363) Bump mockito-core from 3.7.0 to 3.7.7 (JabRef#7360) Bump org.beryx.jlink from 2.23.1 to 2.23.2 (JabRef#7361) Bump libreoffice from 7.0.3 to 7.0.4 (JabRef#7362) Export full month name instead of number in ms office (JabRef#7358) ... # Conflicts: # external-libraries.md # src/main/java/module-info.java # src/main/java/org/jabref/gui/shared/SharedDatabaseLoginDialogViewModel.java
create dialog in fxml and use a background task for import
Will fix #7205 and fix #7206
Scanning:
Importing:
Expanded results: